Repository navigation
preproc: fix infinite loop in paste_tokens with leading/consecutive %+ - #321
agourakis82 wants to merge 3 commits into
Conversation
Fixes netwide-assembler#294. In paste_tokens(), prev_nonspace tracks a pointer to the last non-space, non-paste token to identify the left token for explicit pasting (%+). 1. When a leading %+ was dropped (t == NULL), prev_nonspace remained NULL even after *head was updated to point to the subsequent token. 2. In the unpasted token advancement path, prev_nonspace was conditionally updated by inspecting next instead of tok, so when next was another %+, prev_nonspace was never set to point to tok. As a result, a sequence like "%+s%+t" caused the second %+ to also be treated as a leading %+ (!prev_nonspace == true), resetting nextp to head and setting next to the head of the stream (rewinding to s), creating an infinite loop and hanging NASM.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Add regression fixtures covering leading/consecutive %+ inputs.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Fixes an infinite loop in preprocessor token pasting involving leading or consecutive %+ tokens.
Changes:
- Corrects
prev_nonspacetracking after dropping%+. - Fixes token advancement bookkeeping in
paste_tokens().
| File | Summary |
|---|---|
asm/preproc.c |
Fixes %+ token traversal; add regression fixtures for the reported hangs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…consecutive %+
|
Follow-up regression fixture is now included in |
|
I checked the workflow annotations for my three open PRs. The older #320 and #322 runs did not reach a build or test: GitHub reports that approval expired before the workflows were approved. The #321 run is still awaiting approval. Could a maintainer restart the expired #320/#322 runs and approve the pending #321 run when convenient?
No source or workflow change was made for this diagnosis. CI has not validated these heads yet; the earlier local regression results remain separate evidence. Prepared with OpenAI Codex assistance. |
Fixes #294.
Summary
In
asm/preproc.c,paste_tokens()tracksprev_nonspaceas aToken **pointing to the preceding non-whitespace, non-paste token in the stream, which serves as the left token for explicit token pasting (%+).Two bugs in pointer tracking caused infinite loops on malformed inputs such as
%+s%+t:%+is dropped (t == NULL),*prev_next = tok = nextadvances*headto the token following%+(e.g.s). However,prev_nonspaceremainedNULL, failing to record thatheadnow points to a non-space token.prev_nonspacewas updated by checking whethernextwas non-space/non-paste, rather than whethertokwas:Because
prev_nextwas already advanced to&tok->nextandnextwas the second%+, this check evaluated to false, leavingprev_nonspaceasNULL.Consequently, upon reaching the second
%+,!prev_nonspacewas evaluated as true again. The second%+was mistakenly treated as another leading%+, settingnextp = headand rewindingnextback to*head(s), trapping the while loop in an endless cycle.Fix
%+(!t), updateprev_nonspacetoprev_nextif the new head token is not whitespace/paste.prev_nonspace = prev_nextwhenevertokis not whitespace/paste before advancingprev_next = &tok->next.Verified assembling
%+s%+tand%+ts %+tscompletes cleanly without hanging.